Add differenceWithKey
#542
Conversation
...and define `differenceWith` via `differenceWithKey` Closes #389.
This makes the overlapping case significantly faster.
sjakobi
force-pushed
the
sjakobi/issue389-dWK
branch
from
5539624 to
261ba6d
Compare
| differenceWith f = differenceWithKey (const f) | ||
| {-# INLINE differenceWith #-} | ||
|
|
||
| -- | \(O(n \log m)\) Difference with a combining function. When two equal keys are |
There was a problem hiding this comment.
n is the size of the first map. m is the size of the second map. This is a convention this package uses for many functions. I suspect it was adopted from containers.
There was a problem hiding this comment.
Okay, but I don't think that's necessarily what this implementation does; it was left unchanged from the old one.
There was a problem hiding this comment.
It's not obviously wrong to me at least. If the first map is small, the second is a relatively large superset, and lookup[Cont] takes log(m), we still do n lookups in the larger map. To be fair, we don't start these lookups at the root, so maybe O(n log (m/n)) would be more accurate?!
IMHO these log(size)s are not very useful anyways, since on 64-bit systems we have a maximum tree height of 13, and on 32-bit systems the maximum tree height is 8; and you can still have a map with two entries and full tree height…
There was a problem hiding this comment.
The bounds are given assuming sufficiently uniform hashing, but that's not at all the case for important instances like Int. It's ... a problem. I can't say if n log (m/n) is accurate or not, but it should be something symmetrical!
There was a problem hiding this comment.
I take that back. Maybe not symmetrical. But ... I dunno...
There was a problem hiding this comment.
Maybe not symmetrical actually. I have no idea.
...and define
differenceWithviadifferenceWithKey.Closes #389, closes #364.